Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Statemine state migration runtime changes.#1743

Merged
paritytech-processbot[bot] merged 11 commits into
paritytech:masterfrom
cheme:statemine-mig
Nov 7, 2022
Merged

Statemine state migration runtime changes.#1743
paritytech-processbot[bot] merged 11 commits into
paritytech:masterfrom
cheme:statemine-mig

Conversation

@cheme

@cheme cheme commented Oct 7, 2022

Copy link
Copy Markdown
Contributor

This PR contains runtime change to switch state mine to a hybrid trie state, as needed for https://github.com/paritytech/devops/issues/1508.
Note that the accounts need to be change to the rights KEY_ROOT_MIG_CONTROLLER and KEY_MIG_CONTROLLER that will be responsible for the migration. CC @PierreBesson

@paritytech-ci paritytech-ci requested review from a team October 7, 2022 14:24
Comment thread parachains/runtimes/assets/statemine/src/lib.rs Outdated
Comment on lines +1009 to +1034
pub struct MigController;
pub struct RootMigController;

const KEY_ROOT_MIG_CONTROLLER: [u8; 32] = [
82, 188, 113, 193, 236, 165, 53, 55, 73, 84, 45, 253, 240, 175, 151, 191, 118, 79, 156, 47, 68,
232, 96, 205, 72, 95, 28, 216, 100, 0, 246, 73,
];

const KEY_MIG_CONTROLLER: [u8; 32] = [
82, 188, 113, 193, 236, 165, 53, 55, 73, 84, 45, 253, 240, 175, 151, 191, 118, 79, 156, 47, 68,
232, 96, 205, 72, 95, 28, 216, 100, 0, 246, 73,
];

impl SortedMembers<AccountId> for RootMigController {
fn sorted_members() -> Vec<AccountId> {
// hardcoded key of controller for manual migration
vec![KEY_ROOT_MIG_CONTROLLER.into()]
}
}

impl SortedMembers<AccountId> for MigController {
fn sorted_members() -> Vec<AccountId> {
// hardcoded key of controller for manual migration
vec![KEY_MIG_CONTROLLER.into()]
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub struct MigController;
pub struct RootMigController;
const KEY_ROOT_MIG_CONTROLLER: [u8; 32] = [
82, 188, 113, 193, 236, 165, 53, 55, 73, 84, 45, 253, 240, 175, 151, 191, 118, 79, 156, 47, 68,
232, 96, 205, 72, 95, 28, 216, 100, 0, 246, 73,
];
const KEY_MIG_CONTROLLER: [u8; 32] = [
82, 188, 113, 193, 236, 165, 53, 55, 73, 84, 45, 253, 240, 175, 151, 191, 118, 79, 156, 47, 68,
232, 96, 205, 72, 95, 28, 216, 100, 0, 246, 73,
];
impl SortedMembers<AccountId> for RootMigController {
fn sorted_members() -> Vec<AccountId> {
// hardcoded key of controller for manual migration
vec![KEY_ROOT_MIG_CONTROLLER.into()]
}
}
impl SortedMembers<AccountId> for MigController {
fn sorted_members() -> Vec<AccountId> {
// hardcoded key of controller for manual migration
vec![KEY_MIG_CONTROLLER.into()]
}
}
frame_support::ord_parameter_types! {
pub const KEY_MIG_CONTROLLER = hex!("INSERT_HEX_STRING");
pub const KEY_ROOT_MIG_CONTROLLER = hex!("INSERT_HEX_STRING");
}
const KEY_ROOT_MIG_CONTROLLER: [u8; 32] = [
82, 188, 113, 193, 236, 165, 53, 55, 73, 84, 45, 253, 240, 175, 151, 191, 118, 79, 156, 47, 68,
232, 96, 205, 72, 95, 28, 216, 100, 0, 246, 73,
];
const KEY_MIG_CONTROLLER: [u8; 32] = [
82, 188, 113, 193, 236, 165, 53, 55, 73, 84, 45, 253, 240, 175, 151, 191, 118, 79, 156, 47, 68,
232, 96, 205, 72, 95, 28, 216, 100, 0, 246, 73,
];
impl SortedMembers<AccountId> for RootMigController {
fn sorted_members() -> Vec<AccountId> {
// hardcoded key of controller for manual migration
vec![KEY_ROOT_MIG_CONTROLLER.into()]
}
}
impl SortedMembers<AccountId> for MigController {
fn sorted_members() -> Vec<AccountId> {
// hardcoded key of controller for manual migration
vec![KEY_MIG_CONTROLLER.into()]
}
}

apis: RUNTIME_API_VERSIONS,
transaction_version: 8,
state_version: 0,
state_version: 1,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should feature gate this for now.

As the GCP is still deploying the chain on Rococo for testing.

Assets: pallet_assets::{Pallet, Call, Storage, Event<T>} = 50,
Uniques: pallet_uniques::{Pallet, Call, Storage, Event<T>} = 51,

StateTrieMigration: pallet_state_trie_migration = 52,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also be feature gated.

@paritytech-ci paritytech-ci requested review from a team October 7, 2022 15:30
Comment on lines +1030 to +1042
#[test]
fn ensure_key_ss58() {
use frame_support::traits::SortedMembers;
use sp_core::crypto::Ss58Codec;
let acc =
AccountId::from_ss58check("5DwBmEFPXRESyEam5SsQF1zbWSCn2kCjyLW51hJHXe9vW4xs").unwrap();
//panic!("{:x?}", acc);
assert_eq!(acc, MigController::sorted_members()[0]);
let acc =
AccountId::from_ss58check("5DwBmEFPXRESyEam5SsQF1zbWSCn2kCjyLW51hJHXe9vW4xs").unwrap();
assert_eq!(acc, RootMigController::sorted_members()[0]);
//panic!("{:x?}", acc);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use it to ensure the controller account is correct (from the ss58 one).
I did check again why I did it in the first place, turns out Ss58Codec is behind 'full_crypto' feature and 'from_ss58check' is behind std.
I don't really see why from_ss58check could not be no_std, just would be a bit more work to switch it.

type ControlOrigin = frame_system::EnsureSignedBy<RootMigController, AccountId>;
// specific account for the migration, can trigger the signed migrations.
type SignedFilter = frame_system::EnsureSignedBy<MigController, AccountId>;
//type SignedFilter = frame_system::EnsureSigned<Self::AccountId>;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//type SignedFilter = frame_system::EnsureSigned<Self::AccountId>;

Comment thread parachains/runtimes/assets/statemine/src/lib.rs
Comment thread parachains/runtimes/assets/statemine/src/lib.rs
Comment thread parachains/runtimes/assets/statemine/src/lib.rs
Comment thread parachains/runtimes/assets/statemine/src/lib.rs Outdated
Comment thread parachains/runtimes/assets/statemine/src/lib.rs Outdated
Comment thread parachains/runtimes/assets/statemine/Cargo.toml Outdated
Comment thread parachains/runtimes/assets/statemine/Cargo.toml Outdated
@paritytech-ci paritytech-ci requested a review from a team October 10, 2022 07:30
Comment thread parachains/runtimes/assets/statemine/src/lib.rs Outdated
Comment thread parachains/runtimes/assets/statemine/src/lib.rs
Comment thread parachains/runtimes/assets/statemine/Cargo.toml Outdated

@kianenigma kianenigma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if/why we need a new feature flag.

@paritytech-ci paritytech-ci requested a review from a team October 11, 2022 08:18

@kianenigma kianenigma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please merge #1424 (review) as well before starting the devops process.

@cheme cheme added B0-silent Changes should not be mentioned in any release notes A0-please_review Pull request needs code review. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. C1-low PR touches the given topic and has a low impact on builders. labels Nov 7, 2022
@cheme

cheme commented Nov 7, 2022

Copy link
Copy Markdown
Contributor Author

bot merge

@paritytech-processbot

Copy link
Copy Markdown

Error: Statuses failed for 48642b3

@cheme

cheme commented Nov 7, 2022

Copy link
Copy Markdown
Contributor Author

bot merge

@paritytech-processbot

Copy link
Copy Markdown

Waiting for commit status.

@paritytech-processbot

Copy link
Copy Markdown

Merge cancelled due to error. Error: Statuses failed for 180ab19

@cheme

cheme commented Nov 7, 2022

Copy link
Copy Markdown
Contributor Author

bot merge

@paritytech-processbot

Copy link
Copy Markdown

Waiting for commit status.

@paritytech-processbot paritytech-processbot Bot merged commit 670945d into paritytech:master Nov 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants